Skip to content

Linter: Treat breadcrumb as a side-effecting helper in erb-no-unused-expressions#1845

Open
6temes wants to merge 1 commit into
marcoroth:mainfrom
6temes:allow-breadcrumb-in-unused-expressions
Open

Linter: Treat breadcrumb as a side-effecting helper in erb-no-unused-expressions#1845
6temes wants to merge 1 commit into
marcoroth:mainfrom
6temes:allow-breadcrumb-in-unused-expressions

Conversation

@6temes

@6temes 6temes commented Jul 8, 2026

Copy link
Copy Markdown

What

Adds breadcrumb to SIDE_EFFECT_METHODS in erb-no-unused-expressions, so <% breadcrumb :key %> calls are no longer reported as unused expressions.

Why

gretel — a widely used breadcrumbs gem — exposes breadcrumb as a view helper that is called purely for its side effect. It sets the current breadcrumb trail and is meant to be used in a silent tag:

<% breadcrumb :product, @product %>

Under the hood it just assigns the renderer and returns it:

def breadcrumb(key = nil, *args)
  @_gretel_renderer = Gretel::Renderer.new(self, key, *args)
end

This is the same shape as content_for, provide, and the turbo_* helpers already on the allowlist: a bare, receiver-less call used for a side effect, whose return value is intentionally discarded (you can't meaningfully <%= %> it — that would print a Gretel::Renderer). Today the rule flags every <% breadcrumb %>, so an app that renders breadcrumbs on most pages gets one offense per view, which in practice forces disabling the rule project-wide.

Verification

  • Added a valid cases test mirroring the existing provide / Turbo tests.
  • Behaviorally confirmed against the published 0.10.1 build: with breadcrumb allowlisted, <% breadcrumb :key %> passes while a genuine <% @foo %> is still reported.

I wasn't able to run the full JS test suite locally (the workspace build pulls in the native/Emscripten toolchain), so I'd appreciate CI confirming the added test.

@6temes 6temes marked this pull request as ready for review July 9, 2026 06:01
@6temes 6temes force-pushed the allow-breadcrumb-in-unused-expressions branch from 2c31996 to 0e216dc Compare July 9, 2026 06:01
@brunoprietog

Copy link
Copy Markdown

erb-no-unused-expressions has the potential to become a massive set of allowed expressions, and it actually ignores the fact that you yourself could create helpers that modify instance variables or use the provide helper under the hood. I said that in this comment, using Turbo as an example, but I don't think adding a fixed list of allowed expressions is really the way to go. Having this rule enabled by default basically means prohibiting these kinds of custom helpers, which are sometimes quite useful for dynamically enabling or disabling things, or changing a view’s behavior using natural language. Not all helpers need to use the <%= syntax. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants